Skip to content

Ensure no trailing data for PKCS8 EVP_parse_private_key#3242

Merged
torben-hansen merged 2 commits into
aws:mainfrom
torben-hansen:trailing_fix_pkcs8
May 12, 2026
Merged

Ensure no trailing data for PKCS8 EVP_parse_private_key#3242
torben-hansen merged 2 commits into
aws:mainfrom
torben-hansen:trailing_fix_pkcs8

Conversation

@torben-hansen
Copy link
Copy Markdown
Contributor

Description of changes:

Similar to EVP_parse_public_key(), don't allow trailing data. This matches what EVP_parse_public_key() does.

Testing:

Cover this with a test case.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.29%. Comparing base (25a859c) to head (5003d9c).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3242      +/-   ##
==========================================
+ Coverage   78.12%   78.29%   +0.16%     
==========================================
  Files         689      689              
  Lines      123219   123229      +10     
  Branches    17138    17143       +5     
==========================================
+ Hits        96270    96485     +215     
+ Misses      26038    25831     -207     
- Partials      911      913       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_EVP, EVP_R_DECODE_ERROR));
ERR_clear_error();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also test a V2?

// EVP_parse_private_key must also reject trailing data inside the SEQUENCE
// after the optional [1] publicKey of an RFC 5958 v2 OneAsymmetricKey. This
// pins the invariant that the trailing-data check fires after the optional
// fields are consumed.
TEST(EVPExtraTest, ParsePrivateKeyV2RejectsTrailingData) {
  // Base vector is the v2 OneAsymmetricKey from
  // EVPExtraTest.Ed25519PKCS8v2WithAttributes (empty [0] attributes and a
  // [1] publicKey), with two extra bytes (ASN.1 NULL: 05 00) appended inside
  // the outer SEQUENCE. The outer SEQUENCE length byte is bumped
  // 0x53 -> 0x55 to cover the extra bytes, so the outer TLV is itself
  // well-formed.
  static const uint8_t kDERWithTrailing[] = {
    0x30, 0x55,                                     // SEQUENCE, len 0x55
      0x02, 0x01, 0x01,                             // version = 1 (v2)
      0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x70,     // AlgId { OID 1.3.101.112 }
      0x04, 0x22,                                   // OCTET STRING, len 0x22
        0x04, 0x20,                                 //   OCTET STRING, len 32
        0x9d, 0x61, 0xb1, 0x9d, 0xef, 0xfd, 0x5a, 0x60,
        0xba, 0x84, 0x4a, 0xf4, 0x92, 0xec, 0x2c, 0xc4,
        0x44, 0x49, 0xc5, 0x69, 0x7b, 0x32, 0x69, 0x19,
        0x70, 0x3b, 0xac, 0x03, 0x1c, 0xae, 0x7f, 0x60,
      0xa0, 0x00,                                   // [0] attributes = { }
      0x81, 0x21, 0x00,                             // [1] publicKey BIT STRING
        0xd7, 0x5a, 0x98, 0x01, 0x82, 0xb1, 0x0a, 0xb7,
        0xd5, 0x4b, 0xfe, 0xd3, 0xc9, 0x64, 0x07, 0x3a,
        0x0e, 0xe1, 0x72, 0xf3, 0xda, 0xa6, 0x23, 0x25,
        0xaf, 0x02, 0x1a, 0x68, 0xf7, 0x07, 0x51, 0x1a,
      0x05, 0x00,                                   // trailing ASN.1 NULL
  };

  CBS cbs;
  CBS_init(&cbs, kDERWithTrailing, sizeof(kDERWithTrailing));
  ERR_clear_error();
  bssl::UniquePtr<EVP_PKEY> parsed(EVP_parse_private_key(&cbs));
  EXPECT_FALSE(parsed);
  EXPECT_TRUE(ErrorEquals(ERR_get_error(), ERR_LIB_EVP, EVP_R_DECODE_ERROR));
  ERR_clear_error();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's basically redundant because the strict no trailing data is not an invariant that depends on the version. The sequence exist in both versions.

@torben-hansen torben-hansen merged commit 3671c7a into aws:main May 12, 2026
564 of 566 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants